Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Support managed_draining argument at aws_ecs_capacity_provider #158

Merged
merged 4 commits into from
Feb 12, 2024
Merged

feat: Support managed_draining argument at aws_ecs_capacity_provider #158

merged 4 commits into from
Feb 12, 2024

Conversation

ivan-sukhomlyn
Copy link
Contributor

@ivan-sukhomlyn ivan-sukhomlyn commented Jan 26, 2024 •
edited
Loading

Description

Support of managed_draining argument at auto_scaling_group_provider block of aws_ecs_capacity_provider resource introduced by

Motivation and Context

Breaking Changes

⚠️ Minimal required AWS provider version is >=5.34.0

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects

For already created ECS ASG capacity provider:

  • config
module "ecs_cluster" {
  source = "../../../../../../open-source/terraform-aws-ecs/modules/cluster"

  cluster_name = local.ecs_cluster_name

  # Capacity provider - autoscaling groups
  default_capacity_provider_use_fargate = false

  autoscaling_capacity_providers = {
    for k, v in module.asg_ecs : "${local.ecs_cluster_name}-${k}" => {
      auto_scaling_group_arn         = v.autoscaling_group_arn
      managed_draining               = "ENABLED"
# ....
}
  • Terraform running
$ terraform init -upgrade && terraform plan
...
- Installing hashicorp/aws v5.34.0...
- Installed hashicorp/aws v5.34.0 (signed by HashiCorp)
...


  # module.ecs_cluster.aws_ecs_capacity_provider.this["xxxxx"] will be updated in-place
  ~ resource "aws_ecs_capacity_provider" "this" {
        id       = "arn:aws:ecs:xxx:xxx:capacity-provider/xxxxx"
        name     = "xxxx"

        # (2 unchanged attributes hidden)

      ~ auto_scaling_group_provider {
          ~ managed_draining               = "DISABLED" -> "ENABLED"
            # (2 unchanged attributes hidden)

            # (1 unchanged block hidden)
        }
    }
  • I have executed pre-commit run -a on my pull request
✗ pre-commit run -a
Terraform fmt............................................................Passed
Terraform wrapper with for_each in module................................Passed
Terraform validate.......................................................Passed
Terraform docs...........................................................Passed
Terraform validate with tflint...........................................Passed
check for merge conflicts................................................Passed
fix end of files.........................................................Passed

@ivan-sukhomlyn ivan-sukhomlyn changed the title feat: Support managed_draining argument at aws_ecs_capacity_provider resource feat: Support managed_draining argument at aws_ecs_capacity_provider Jan 26, 2024
bryantbiggs
bryantbiggs requested changes Jan 26, 2024
View reviewed changes
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a breaking change - we have a few other breaking changes batched up now so we should do them all at once

also - the version would need to be aligned across the project

@ivan-sukhomlyn
Copy link
Contributor Author

@bryantbiggs should I do something additionally from my side in that case?

@bryantbiggs bryantbiggs added this to the v6.0.0 milestone Jan 26, 2024
@remiflament
Copy link

yes @ivan-sukhomlyn the versions.tf should be the same in all the project. So it needs to be update in all the other submodules.

@ivan-sukhomlyn
Copy link
Contributor Author

oops!
@remiflament thank you for the clarification 👍 🙂

@bryantbiggs bryantbiggs changed the base branch from master to wip/v6 February 12, 2024 15:33
bryantbiggs
bryantbiggs approved these changes Feb 12, 2024
View reviewed changes
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Merging this into the wip/v6 branch

@bryantbiggs bryantbiggs merged commit 2f04bba into terraform-aws-modules:wip/v6 Feb 12, 2024
2 checks passed
@ivan-sukhomlyn ivan-sukhomlyn mentioned this pull request Feb 12, 2024
Closed
@ivan-sukhomlyn ivan-sukhomlyn deleted the feat/managed_draining branch February 12, 2024 16:13
@easterbonbon easterbonbon mentioned this pull request Feb 27, 2024
Closed
@github-actions GitHub Actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bryantbiggs bryantbiggs bryantbiggs approved these changes

Assignees
No one assigned
Labels
breaking change enhancement
Projects
None yet
Milestone
v6.0.0
Development

Successfully merging this pull request may close these issues.

feat: add managed_draining to aws_ecs_capacity_provider
3 participants
@ivan-sukhomlyn @remiflament @bryantbiggs